apps/portal/src/app/views/applications/applications.service.ts (1)

Line range hint 22-38: Use the common error handler for consistency.

The service should use the new validateApolloErrors method for consistent error handling across the application.

Apply this change:

   getApplications(variables: unknown, inputToken: string): Observable<ApplicationResponse> {
     return this.graphqlService.query(GetApplications, variables, inputToken).pipe(
       map((response: ApolloQueryResult<GetApplicationsResponse>) => {
+        this.graphqlService.validateApolloErrors(response);
         const applicationArray = response.data?.applications.applications;

         const applicationResponseObject: ApplicationResponse = {
           applications: applicationArray,
           total: response.data?.applications.total,
           offset: response.data?.applications.offset,
           limit: response.data?.applications.limit,
-          errors: response.errors || null,  // Remove this as errors are now handled by validateApolloErrors
         };
         return applicationResponseObject;
       }),
       catchError((error) => {
         const errorMessage: string = error.message;
         throw new Error(errorMessage);
       }),
     );
   }

apps/portal/src/app/views/providers/providers.service.ts (2)

48-95: Consider extracting common code for the archived vs non-archived branches.
The if (archivedNotificationToggle) branch duplicates logic (mapping, error handling, etc.). You could reduce duplication by consolidating transformations in a helper function.

Sample approach:

if (archivedNotificationToggle) {
  // ...
} else {
  // ...
}

+ // Potentially unify repeated logic in a shared callback that converts 
+ // providers and notifications (or archived notifications) to the standard form 
+ // and handles the error and pipe. 

---

97-120: Catch error objects without losing context.
When catching errors, re-throwing them as new Error objects can lose stack traces or additional info. Consider re-throwing the original error or attaching the original data to preserve debugging context.

apps/portal/src/app/views/notifications/notifications.component.ts (3)

48-48: Initialize providers with stronger type.
If you intend providers to always hold an array of typed provider objects, explicitly declare the array type to improve maintainability and clarity.

- providers = [];
+ providers: { label: string; value: number }[] = [];

---

Line range hint 102-179: Avoid deeply nested subscriptions in production code.
While this is acceptable, consider organizing load steps so each call unsubscribes properly or merges streams elegantly (e.g., using RxJS switchMap) to reduce the chance of memory leaks.

---

384-386: Filters in multiple places.
You have multiple filter sets (in loadProvidersAndNotificationsForSelectedApplication and in loadNotificationsLazy). Consider centralizing or reusing filter logic to avoid duplication.

apps/portal/src/app/views/providers/provider.model.ts (1)

1-11: Prefer enumerations where applicable.
channelType and status are typed as number; for maintainability, consider using a TypeScript enum or union for clarity and safer references.

apps/portal/src/app/graphql/graphql.service.ts (1)

63-73: Consider enhancing error types and handling.

The error handler implementation is good but could be improved with more specific error types and additional context.

Consider these enhancements:

-  validateApolloErrors<T>(response: ApolloQueryResult<T>): void {
+  validateApolloErrors<T>(response: ApolloQueryResult<T>): void {
+    interface GraphQLError {
+      code?: string;
+      message: string;
+      path?: readonly (string | number)[];
+    }
+
     if (response.errors && response.errors.length > 0) {
-      const errorMessages = response.errors.map((err) => err.message).join('; ');
+      const errorMessages = response.errors.map((err: GraphQLError) => 
+        `${err.code ? `[${err.code}] ` : ''}${err.message}${err.path ? ` at ${err.path.join('.')}` : ''}`
+      ).join('; ');
       throw new Error(`GraphQL Formatted Error(s): ${errorMessages}`);
     }

     if (response.error) {
-      throw new Error(`Apollo Error: ${response.error.message}`);
+      throw new Error(`Apollo Network Error: ${response.error.message}`);
     }
   }

apps/portal/src/app/graphql/graphql.queries.ts (2)

97-154: Consider using GraphQL fragments to reduce duplication.

The provider and notification field selections are duplicated across queries. Using fragments would improve maintainability.

Consider refactoring to use fragments:

+const ProviderFields = gql`
+  fragment ProviderFields on Provider {
+    applicationId
+    channelType
+    createdOn
+    name
+    providerId
+    status
+    updatedOn
+  }
+`;
+
+const NotificationFields = gql`
+  fragment NotificationFields on Notification {
+    channelType
+    createdBy
+    createdOn
+    data
+    deliveryStatus
+    id
+    result
+    status
+    updatedBy
+    updatedOn
+  }
+`;

 export const GetProvidersAndNotifications = gql`
+  ${ProviderFields}
+  ${NotificationFields}
   query GetProvidersAndNotifications(
     $providerLimit: Int!
     # ... other variables
   ) {
     providers(
       options: {
         # ... options
       }
     ) {
       providers {
-        applicationId
-        channelType
-        # ... other fields
+        ...ProviderFields
       }
       # ... pagination fields
     }
     notifications {
       notifications {
-        channelType
-        createdBy
-        # ... other fields
+        ...NotificationFields
       }
       # ... pagination fields
     }
   }
 `;

---

156-213: Consider making sort order configurable.

The sort orders are hardcoded (ASC for providers, DESC for notifications). Consider making them configurable via variables.

Consider this enhancement:

 export const GetProvidersAndArchivedNotifications = gql`
   query GetProvidersAndArchivedNotifications(
     $providerLimit: Int!
     $providerOffset: Int!
     $providerFilters: [UniversalFilter!]
+    $providerSortOrder: SortOrder = ASC
     $notificationLimit: Int!
     $notificationOffset: Int!
     $notificationFilters: [UniversalFilter!]
+    $notificationSortOrder: SortOrder = DESC
   ) {
     providers(
       options: {
         limit: $providerLimit
         offset: $providerOffset
         sortBy: "createdOn"
-        sortOrder: ASC
+        sortOrder: $providerSortOrder
         filters: $providerFilters
       }
     )
     # ... rest of the query
   }
 `;

apps/portal/src/app/views/notifications/notifications.service.ts (1)

Line range hint 32-83: Extract common error handling logic.

Both methods have identical error handling logic. Consider extracting it to a private method.

Consider this refactor:

+  private handleGraphQLError(error: Error): never {
+    const errorMessage: string = error.message;
+    throw new Error(errorMessage);
+  }
+
   getNotifications(variables: unknown, inputToken: string): Observable<NotificationResponse> {
     return this.graphqlService.query(GetNotifications, variables, inputToken).pipe(
       map((response: ApolloQueryResult<GetNotificationsResponse>) => {
         this.graphqlService.validateApolloErrors(response);
         // ... rest of the mapping logic
       }),
-      catchError((error) => {
-        const errorMessage: string = error.message;
-        throw new Error(errorMessage);
-      }),
+      catchError(this.handleGraphQLError.bind(this)),
     );
   }

   getArchivedNotifications(variables: unknown, inputToken: string): Observable<NotificationResponse> {
     return this.graphqlService.query(GetArchivedNotifications, variables, inputToken).pipe(
       map((response: ApolloQueryResult<GetArchivedNotificationsResponse>) => {
         this.graphqlService.validateApolloErrors(response);
         // ... rest of the mapping logic
       }),
-      catchError((error) => {
-        const errorMessage: string = error.message;
-        throw new Error(errorMessage);
-      }),
+      catchError(this.handleGraphQLError.bind(this)),
     );
   }

apps/portal/src/app/views/notifications/notifications.component.html (1)

55-57: LGTM! Consider caching providers.

The change to load providers and notifications together is a good improvement. However, since providers data might not change as frequently as notifications, consider caching the providers data to reduce unnecessary API calls.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro